fix(omlx): keep role thinking effort on discovered models - #4631

Adversarial red-team review at exact head d4ea42200f088620477be87eebabd5253f69d7b8 (base 8711c555da490ad36475f94368bcdd8d8f01048e, one commit, 9 files, +129/−4). Reviewed against current dev f3726ef05e507c9e0123fadbe17af833d1b1e7e3.

Verdict: MERGE_READY

Focused local verification (detached read-only worktree at d4ea422, review branch untouched)

bun test packages/coding-agent/test/omlx-discovery.test.ts \
          packages/coding-agent/test/model-profiles-catalog.test.ts \
          packages/ai/test/openai-completions-compat.test.ts \
          packages/ai/test/model-thinking.test.ts \
          packages/coding-agent/test/model-registry.test.ts
→ 365 pass / 0 fail / 4713 expect() calls

Notes on the run: the temp worktree needed bun --cwd=packages/natives run build (fresh checkout ships no addon; the global copies carry an older __piNativesV sentinel). Three model-registry.test.ts cases initially failed because this host exports ANTHROPIC_BASE_URL/OPENAI_BASE_URL pointing at a proxy — pre-existing test/env interaction, reproduces identically without the PR patch; after env -u of those vars all 365 pass. Exact-head CI run 32026121902 is green on all substantive affected-path, package, and evidence checks; only the needs-human contract jobs are red (PR body carries the verdict request), which this review addresses.

Findings (no blockers)

1. Bug confirmed real at base. At 8711c55, discovery (model-registry.ts:3042-3073) synthesized oMLX rows with reasoning: referenceModel?.reasoning ?? false and hardcoded supportsReasoningEffort: false, so macos-omlx-* role suffixes (:low/:medium/:high from #4607's model-profiles.ts:103-135) were clamped away before request generation. The patch fixes both sides.
2. Test allowlist matches the #4607 profiles exactly. macosOmlxModels in model-profiles-catalog.test.ts:688-693 enumerates the same five ids the five built-in profiles reference (Qwen3.6-35B-A3B-{4bit,8bit,bf16}, Qwen3.8-27B-Abliterated-MLX-{4bit,6bit}) — verified against model-profiles.ts:103-135 at base. It is test-harness-only (selector validation); runtime discovery still accepts any served oMLX id, so non-Qwen weights served via oMLX are not excluded, they just may not honor effort.
3. Effort normalization and omission semantics are correct. getSupportedEfforts → explicit thinking {min: low, max: high, default: medium} (expandEffortRange yields low/medium/high); clampThinkingLevelForModel clamps xhigh/max → high and minimal → low per the floor rule. Catalog-refresh path (applyGeneratedModelPolicies + inferFallbackEfforts/inferDefaultEffort in model-thinking.ts) is forced-consistent for oMLX regardless of what /v1/models reports. On the wire (openai-completions.ts:1400-1407), enable_thinking is always emitted (false when reasoning is off/disabled — identical to base behavior), and reasoning_effort is added only when thinking is enabled and an effort was requested; no stale/minimal effort leaks into disabled turns. oMLX defines no reasoningEffortMap, so low/medium/high pass through verbatim.
4. Non-oMLX behavior unchanged. Every new provider-scoped branch in model-thinking.ts, openai-compat.ts, and model-registry.ts is gated on provider === "omlx" (the applyGeneratedModelPolicy one additionally on api === "openai-completions"). git grep at head shows no bundled model or provider config outside oMLX uses thinkingFormat: "qwen-chat-template", so the buildParams change is scoped to oMLX plus any user who explicitly sets that format — for whom adding reasoning_effort only when effort is present is a strict improvement, not a regression. github-copilot gate, zai/qwen/openrouter branches untouched.
5. Duplication is deliberate, not drift. Discovery-time defaults (model-registry.ts / mapModel in openai-compat.ts) and catalog-refresh enforcement (applyGeneratedModelPolicies) encode the same capability tuple; the refresh path is the safety net if a models.dev refresh rewrites oMLX rows. Values are identical across all three sites (reasoning true, low..high, medium default, qwen-chat-template, reasoning_content, no store, no developer role).
6. Current-dev interaction: none. f3726ef changes only workflow-gate-broker.ts, its two test files, and a changelog — disjoint from every file this PR touches. Clean merge; no semantic overlap in thinking/model-registry paths.
7. Security/privacy: no new secret handling, logging, or URL surface; discovery error redaction (redactDiscoveryUrl) unchanged; the oMLX token path is untouched.

Residual risk (accepted, non-blocking)

• No live oMLX dogfood was run (Linux host; oMLX is macOS-only). The patch assumes the served endpoint accepts chat_template_kwargs.reasoning_effort. #4607 already committed this transport to chat_template_kwargs for enable_thinking, and the Qwen3 chat template documents the effort kwarg, so the assumption is well-grounded; a backend that ignores unknown kwargs degrades gracefully to #4607 behavior.
• No explicit test for the enable_thinking: false + effort-present combination, but the code path structurally omits reasoning_effort there and existing tests cover the enabled path end-to-end.

Merge was not performed; this is review-only per the assignment.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

Exact-head approval at rebased head 4f70373.

Re-verification of review 4953113614 (MERGE_READY at d4ea422): the rebase onto dev f3726ef is content-identical to the approved patch (zero-line delta excluding index hashes); re-ran locally at 4f70373 — bun test omlx-discovery/model-profiles-catalog/openai-completions-compat/model-thinking/model-registry: 365 pass / 0 fail; bun --cwd=packages/ai run check: pass; bun --cwd=packages/coding-agent run check: pass; fast gate (verify-gjc-state-writers --fail): pass; base f3726ef is ancestor; PR files disjoint from dev f3726ef delta (workflow-gate-broker only). Canonical diff digest unchanged: 53f2e0aa18fa88a404796b827db18afe6755e257b8dfb3b0b5d41ecc766e1c64.

No merge performed yet; this approval enables the exact-head contract transition.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

Contract transition record: verdict line updated needs-human → merge-approved (digest unchanged 53f2e0aa18fa88a404796b827db18afe6755e257b8dfb3b0b5d41ecc766e1c64, content-identical rebase), fresh exact-head approval 4953649002 at 4f70373. This comment re-triggers the review-event contract run against the current body.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]